Skip to content

cks: fix list apis response count#8701

Merged
kiranchavala merged 2 commits intoapache:4.19from
shapeblue:cks-list-count
Jun 13, 2024
Merged

cks: fix list apis response count#8701
kiranchavala merged 2 commits intoapache:4.19from
shapeblue:cks-list-count

Conversation

@shwstppr
Copy link
Contributor

Description

Fixes count value in listKubernetesClusters and listSupportedKubernetesVersions APIs response.
Currently, it returns items count of the response

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@shwstppr shwstppr changed the title cks: fix list apis count cks: fix list apis response count Feb 23, 2024
Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

@blueorangutan
Copy link

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 8762

@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@codecov
Copy link

codecov bot commented Feb 23, 2024

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 14.96%. Comparing base (cadbb56) to head (73e61b4).
Report is 54 commits behind head on 4.19.

Files Patch % Lines
...bernetes/cluster/KubernetesClusterManagerImpl.java 0.00% 4 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##               4.19    #8701       +/-   ##
=============================================
- Coverage     15.13%   14.96%    -0.18%     
+ Complexity    13469    10987     -2482     
=============================================
  Files          4863     5373      +510     
  Lines        326031   469026   +142995     
  Branches      45838    58668    +12830     
=============================================
+ Hits          49349    70177    +20828     
- Misses       270066   391082   +121016     
- Partials       6616     7767     +1151     
Flag Coverage Δ
simulator-marvin-tests ?
uitests 4.31% <ø> (?)
unittests 15.66% <63.63%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8763

@shwstppr
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-9324)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40751 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8701-t9324-kvm-centos7.zip
Smoke tests completed. 109 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_08_migrate_vm Error 0.05 test_vm_life_cycle.py

Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@shwstppr shwstppr marked this pull request as ready for review February 26, 2024 04:47
Fixes count value in listKubernetesClusters and
listSupportedKubernetesVersions APIs response.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9310

@shwstppr shwstppr closed this May 24, 2024
@shwstppr shwstppr reopened this May 24, 2024
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9698

@shwstppr
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

@shwstppr a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

[SF] Trillian test result (tid-10282)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45618 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8701-t10282-kvm-centos7.zip
Smoke tests completed. 130 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File
test_02_trigger_shutdown Failure 341.75 test_safe_shutdown.py

@kiranchavala kiranchavala self-assigned this Jun 12, 2024
Copy link
Member

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shwstppr Just tested on the latest 4.19, I found that count for both apis is working fine

Any step I am missing?

shapeblue) 🐱 > list kubernetessupportedversions filter=name
{
  "count": 3,
  
  
  (shapeblue) 🐱 > list kubernetesclusters filter=name
{
  "count": 2,
  

@shwstppr
Copy link
Contributor Author

@kiranchavala can you please try giving a pagesize lesser than the number of resources present in the env?

@shwstppr
Copy link
Contributor Author

shwstppr commented Jun 12, 2024

@kiranchavala here is an example in an env without fix (count should have been returned same [=3] in both case),

(local) 🐱 > list kubernetessupportedversions pagesize=2 page=1 filter=id,name
{
  "count": 2,
  "kubernetessupportedversion": [
    {
      "id": "6570218c-7d3f-478c-bb72-1c3c778aaf45",
      "name": "v1.26.0"
    },
    {
      "id": "b3e308be-e39b-4d9c-82d5-de41da9ed843",
      "name": "v1.24.0"
    }
  ]
}
(local) 🐱 > list kubernetessupportedversions filter=id,name
{
  "count": 3,
  "kubernetessupportedversion": [
    {
      "id": "6570218c-7d3f-478c-bb72-1c3c778aaf45",
      "name": "v1.26.0"
    },
    {
      "id": "b3e308be-e39b-4d9c-82d5-de41da9ed843",
      "name": "v1.24.0"
    },
    {
      "id": "04c74e5c-7802-460e-92b7-cce2b8efc546",
      "name": "v1.24.0"
    }
  ]
}

Copy link
Member

@weizhouapache weizhouapache left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

Copy link
Member

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested the fix

The count is same

(cmk) 🐱 > list kubernetessupportedversions filter=id,name
{
  "count": 3,
  "kubernetessupportedversion": [
    {
      "id": "ed372000-c2cb-45e0-aa72-e0eda51f618d",
      "name": "v1.28.4"
    },
    {
      "id": "7f4a657a-96d3-4ed6-afc7-379361299462",
      "name": "v1.27.8"
    },
    {
      "id": "5b2594ec-f41f-4bd4-9ff1-57c1c9a18c20",
      "name": "v1.26.0"
    }
  ]
}


(cmk) 🐱 > list kubernetessupportedversions pagesize=2 page=1 filter=id,name
{
  "count": 3,
  "kubernetessupportedversion": [
    {
      "id": "ed372000-c2cb-45e0-aa72-e0eda51f618d",
      "name": "v1.28.4"
    },
    {
      "id": "7f4a657a-96d3-4ed6-afc7-379361299462",
      "name": "v1.27.8"
    }
  ]
}


(cmk) 🐱 > list kubernetesclusters filter=id,name
{
  "count": 2,
  "kubernetescluster": [
    {
      "id": "21975df3-2f19-4865-890c-e9eb58b671fb",
      "name": "cks1"
    },
    {
      "id": "9607e862-a082-4c5b-a92c-245cf78ca3f0",
      "name": "cks2"
    }
  ]
}

(cmk) 🐱 > list kubernetesclusters pagesize=1 page=1 filter=id,name
{
  "count": 2,
  "kubernetescluster": [
    {
      "id": "21975df3-2f19-4865-890c-e9eb58b671fb",
      "name": "cks1"
    }
  ]
}

@kiranchavala kiranchavala merged commit 2fef0a3 into apache:4.19 Jun 13, 2024
@DaanHoogland DaanHoogland deleted the cks-list-count branch June 13, 2024 07:49
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 17, 2024
* cks: fix list apis count

Fixes count value in listKubernetesClusters and
listSupportedKubernetesVersions APIs response.
@DaanHoogland DaanHoogland added this to the 4.19.2.0 milestone Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants